Fix #9/#12: robust wrap_kvs transform signature-conditioning + FirstArgIsMapping - #72
Merged
Merged
Conversation
…rgIsMapping wrap_kvs decided whether to call a transform as f(data) vs f(self, data) purely by the first parameter NAME (self/store/mapping). This misfired on unary callables whose first param happens to be named 'self' -- e.g. bytes.decode -> 'descriptor decode ... doesn't apply to a Store object' (Issue #9). - _has_unbound_self now ALSO requires >=2 required positional params, so unary builtins like bytes.decode / str.upper are correctly treated as f(data). - Wire in the previously-dead FirstArgIsMapping(LiteralVal) marker as the explicit opt-in (Issue #12), via a single resolver _resolve_self_convention used by all 4 call sites (_wrap_outcoming, _wrap_ingoing, postget, preset). Exported from dol. - Regression tests (tests/test_trans.py) + doctests. - Dev skill dol-dev-wrap-kvs documents the machinery, traps (#18/#6), and the test-gate. Backward-compatible, validated exhaustively: - AST scan of dol + all 76 local dependents: 0 behavior-changing call sites (12 genuine self-convention transforms, all >=2 required, preserved). - Recall-gap scan of attribute/imported-name transforms across 13 heavy users: 0 more. - Dependents test-gate (baseline vs modified, 25 repos): 0 pass->fail regressions. - 6-lens adversarial review: CLEAN (fix even resolves several previously-crashing cases). Closes #9 Closes #12 Claude-Session: https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM
thorwhalen
added a commit
that referenced
this pull request
Jul 4, 2026
Keeps the analysis docs, the consumer skill (dol-store-building), and the CLAUDE index on this branch; the wrap_kvs code change and its dev skill are reviewed separately in PR #72 to keep that behavior change independently reviewable. Claude-Session: https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM
This was referenced Jul 4, 2026
thorwhalen
added a commit
that referenced
this pull request
Jul 4, 2026
* docs: dol architecture map + issues triage/tackle-order report Adds two code-verified study docs under misc/docs/: - dol_architecture_map.md: module map, internal dependency graph, public API surface, class hierarchy, and a deep dive on the wrap_kvs/store_decorator/codec machinery (incl. the exact signature-conditioning logic behind #9), plus a ranked tech-debt list and notes for dev-skill authors. - dol_issues_report.md: prioritized triage — which open issues are already resolved (#40/#50/#52/#58) and a wave-by-wave tackle order for the rest. Also refreshes issues_and_discussions.md (corrects the #9 root-cause: it is first-parameter-*name* matching, not arg-count; marks Windows/caching issues resolved) and updates the docs index. A local-only ecosystem dependents+usage inventory is generated under misc/data/ (gitignored). Claude-Session: https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM * docs: correct #50 framing (was already closed 2025-10-10, not open) Claude-Session: https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM * Fix #9/#12: robust wrap_kvs transform signature-conditioning + FirstArgIsMapping wrap_kvs decided whether to call a transform as f(data) vs f(self, data) purely by the first parameter NAME (self/store/mapping). This misfired on unary callables whose first param happens to be named 'self' -- e.g. bytes.decode -> TypeError. - _has_unbound_self now also requires >=2 required positional params, so unary builtins like bytes.decode/str.upper are correctly treated as f(data) (#9). - Add explicit opt-in marker FirstArgIsMapping (wired in, was dead code) and a single resolver _resolve_self_convention used by all 4 call sites (_wrap_outcoming, _wrap_ingoing, postget, preset) (#12). Exported from dol. - Regression tests in tests/test_trans.py; new doctests. Backward-compatible: AST scan of dol + 76 dependents found 0 behavior-changing call sites; dependents test-gate (baseline vs modified) showed 0 pass->fail regressions. Also adds dev skill dol-dev-wrap-kvs (machinery + traps + test-gate) and consumer skill dol-store-building, and indexes them + the new misc/docs in CLAUDE.md. Refs #9 #12 #18 #6 Claude-Session: https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM * docs: mark #9/#12 resolved in architecture map §5.4 + issues report Claude-Session: https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM * study branch: docs-only (code fix #9/#12 + dev skill live in PR #72) Keeps the analysis docs, the consumer skill (dol-store-building), and the CLAUDE index on this branch; the wrap_kvs code change and its dev skill are reviewed separately in PR #72 to keep that behavior change independently reviewable. Claude-Session: https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the long-standing
wrap_kvsinconsistency (#9) and wires in the proposedFirstArgIsMappingmarker (#12).Problem
wrap_kvsdecided whether to call a transform asf(data)orf(self, data)purely by the transform's first parameter name (self/store/mapping). Soobj_of_data=bytes.decodecrashed (descriptor 'decode' ... doesn't apply to a 'Store' object) whileobj_of_data=lambda x: x.decode()worked — same function, different behavior.Fix
_has_unbound_selfnow requires both first-param-name ∈{self,store,mapping}and ≥2 required positional params. Unary builtins likebytes.decode/str.upperare correctly treated asf(data); genuinedef f(self, data)transforms are unchanged.FirstArgIsMapping(LiteralVal)marker is wired in as the explicit opt-in, via one resolver_resolve_self_conventionused at all four call sites (_wrap_outcoming,_wrap_ingoing,postget,preset). Exported fromdol.Backward-compatibility — validated exhaustively
def f(self),partial(f, data=1)).Tests
8 regression tests in
tests/test_trans.py+ doctests. Full suite green (111 passed).Notes
self) andStore.wrapbreaks the signature of subclasses. #6 (Store.wrapsignature) — those are separate mechanisms, deferred.dol-dev-wrap-kvsdev skill documenting the machinery + the mandatory dependents test-gate.Closes #9, closes #12.
https://claude.ai/code/session_01UpwjDSfDpXWjSgSSubiEiM